34 arc f5 save slack channels #59

NikolayNN · 2017-12-25T11:27:43Z

connect #34

…tabase by a schedule

ivshapovalov · 2017-12-26T11:49:20Z

slack-archive/src/main/java/juja/microservices/slack/archive/model/dto/ChannelDTO.java

+        "id",
+        "name"
+})
+@Getter


Maybe @DaTa instead of many Annotations?

hamster4n · 2017-12-26T14:01:14Z

...archive/src/main/java/juja/microservices/slack/archive/service/impl/SlackApiServiceImpl.java

+@Service
+public class SlackApiServiceImpl implements SlackApiService {
+
+    SlackApi slackApi;


hamster4n · 2017-12-26T14:14:42Z

slack-archive/src/main/java/juja/microservices/slack/archive/model/entity/Channel.java

+@Setter
+@NoArgsConstructor
+@ToString
+@EqualsAndHashCode


need @entity annotation

hamster4n · 2017-12-26T14:17:34Z

slack-archive/src/main/java/juja/microservices/slack/archive/model/entity/Message.java

@@ -1,4 +1,4 @@
-package juja.microservices.slack.archive.model;
+package juja.microservices.slack.archive.model.entity;


need @entity annotation

i read spring docs. MongoTemplate doesn't require @entity annotation

9.5. Saving, Updating, and Removing Documents
https://docs.spring.io/spring-data/mongodb/docs/current/reference/html/#mongo-template

and there are two ways with Id annotation.

A property or field annotated with @id (org.springframework.data.annotation.Id) will be mapped to the _id field.

A property or field without an annotation but named id will be mapped to the _id field.

I think it is a good case to use @id annotation explicitly

I agree with Nikolay

hamster4n · 2017-12-26T14:31:43Z

slack-archive/src/test/java/juja/microservices/slack/utils/Util.java

+
+public class Util {
+
+    public static String getFile(String fileName){


maybe "readStringFromFile" ?

…thod

danilkuznetsov · 2018-01-03T21:04:38Z

slack-archive/src/main/java/juja/microservices/slack/archive/Application.java


 @SpringBootApplication
+@EnableScheduling


Please, move @EnableScheduling to SchedulerConfig class
It will be better

danilkuznetsov · 2018-01-03T21:14:17Z

slack-archive/src/main/java/juja/microservices/slack/archive/model/entity/Message.java

@@ -1,4 +1,4 @@
-package juja.microservices.slack.archive.model;
+package juja.microservices.slack.archive.model.entity;


I agree with Nikolay

danilkuznetsov · 2018-01-03T21:17:55Z

slack-archive/src/main/java/juja/microservices/slack/archive/repository/ArchiveRepository.java

-import juja.microservices.slack.archive.model.Channel;
-import juja.microservices.slack.archive.model.Message;
+import juja.microservices.slack.archive.model.entity.Channel;
+import juja.microservices.slack.archive.model.entity.Message;

 import java.util.List;

 public interface ArchiveRepository {


Maybe better extract logic for messages to another repository
and rename this repo to slackchannelrepository ?

what do you think?

Oh you suggest it too) ok I will do it

danilkuznetsov · 2018-01-03T21:21:39Z

slack-archive/src/main/java/juja/microservices/slack/archive/api/SlackApiImpl.java

+
+@Repository
+@Slf4j
+public class SlackApiImpl implements SlackApi {


Maybe is better naming for interface SlackApiClient?

danilkuznetsov · 2018-01-03T21:23:22Z

slack-archive/src/main/java/juja/microservices/slack/archive/api/SlackApiImpl.java

+
+    @Override
+    public List<ChannelDTO> receiveChannelsList() {
+        String urlTemplate = slackApiChannelsUrlTemplate + slackApiToken;


Old service use simpleslackapi,
Why do you use resttemplate instead?

I will sort out with simpleSlackApi

danilkuznetsov · 2018-01-03T21:23:42Z

slack-archive/src/main/java/juja/microservices/slack/archive/model/entity/Channel.java


 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
-import lombok.Getter;
+import lombok.*;


No * import

danilkuznetsov · 2018-01-03T21:27:47Z

slack-archive/src/main/java/juja/microservices/slack/archive/service/ArchiveService.java

-import juja.microservices.slack.archive.model.MessagesRequest;
+import juja.microservices.slack.archive.model.dto.ChannelDTO;
+import juja.microservices.slack.archive.model.dto.MessagesRequest;
+import juja.microservices.slack.archive.model.entity.Message;

 import java.util.List;

 public interface ArchiveService {


Maybe is better naming ChannelService and extract some messages logic to another service

Yes, it is a good idea and in this case I will do separate repositories for channel and messages

danilkuznetsov · 2018-01-03T21:36:09Z

slack-archive/src/test/java/juja/microservices/slack/utils/Util.java

+
+        String result = "";
+
+        Foo foo = new Foo();


the strange logic for reading the file
For example, you can use in test
IOUtils.toString(this.getClass().getClassLoader().getResourceAsStream(fileName));

the method is static, I can't use non static method this.getClass() here

I want to return this code into test or another case classz should be ones from parameters this method
After that this method becomes simple

danilkuznetsov · 2018-01-03T21:37:04Z

@NikolayNN please update your branch from upstream and fix conflicts

…lack-channels # Conflicts: # slack-archive/src/main/java/juja/microservices/slack/archive/SlackArchiveApplication.java

…o ChannelService, MessageService and ChannelRepository, MessageRepository

NikolayNN added 5 commits December 25, 2017 13:24

JujaLabs#34 get slack channels with slack api

e15c462

JujaLabs#34 added channelDTO

4dc9d71

JujaLabs#34 save channels to the database

60d5633

JujaLabs#34 implemented feature get and save/update channel in the da…

b61dfbf

…tabase by a schedule

JujaLabs#34 renamed and move to another package channel entity

7538449

Garazd added the in progress label Dec 25, 2017

ivshapovalov reviewed Dec 26, 2017

View reviewed changes

hamster4n reviewed Dec 26, 2017

View reviewed changes

JujaLabs#34 some code format, use @DaTa for model classes, renamed me…

cb84d46

…thod

hamster4n previously approved these changes Dec 26, 2017

View reviewed changes

hamster4n assigned hamster4n and unassigned hamster4n Dec 27, 2017

petrokramar added the backend-slack-archive label Dec 28, 2017

danilkuznetsov reviewed Jan 3, 2018

View reviewed changes

JujaLabs#34 renamed from SlackAPI to SlackAPIClient

6c1611a

NikolayNN dismissed hamster4n’s stale review via 6c1611a January 8, 2018 17:20

NikolayNN added 4 commits January 8, 2018 20:31

Merge remote-tracking branch 'upstream/develop' into 34-ARC-F5-save-s…

5860ad2

…lack-channels # Conflicts: # slack-archive/src/main/java/juja/microservices/slack/archive/SlackArchiveApplication.java

JujaLabs#34: optimized imports

19cc064

JujaLabs#34: divided SlackArchiveService and SlackArchiveRepository t…

4261e05

…o ChannelService, MessageService and ChannelRepository, MessageRepository

JujaLabs#34: save rawChannel by schedule

77b73dd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

34 arc f5 save slack channels #59

34 arc f5 save slack channels #59

NikolayNN commented Dec 25, 2017

ivshapovalov Dec 26, 2017

NikolayNN Dec 26, 2017

hamster4n Dec 26, 2017

NikolayNN Dec 26, 2017

hamster4n Dec 26, 2017

hamster4n Dec 26, 2017

NikolayNN Dec 27, 2017 •

edited

Loading

danilkuznetsov Jan 3, 2018

hamster4n Dec 26, 2017

NikolayNN Dec 26, 2017

danilkuznetsov Jan 3, 2018

NikolayNN Jan 6, 2018

danilkuznetsov Jan 3, 2018

danilkuznetsov Jan 3, 2018

NikolayNN Jan 8, 2018 •

edited

Loading

NikolayNN Jan 8, 2018

danilkuznetsov Jan 3, 2018

NikolayNN Jan 6, 2018

danilkuznetsov Jan 3, 2018

NikolayNN Jan 8, 2018

danilkuznetsov Jan 3, 2018

NikolayNN Jan 8, 2018

danilkuznetsov Jan 3, 2018

NikolayNN Jan 8, 2018

NikolayNN Jan 8, 2018

danilkuznetsov Jan 3, 2018

NikolayNN Jan 6, 2018

danilkuznetsov Jan 6, 2018

danilkuznetsov commented Jan 3, 2018 •

edited

Loading

		@@ -1,4 +1,4 @@
		package juja.microservices.slack.archive.model;
		package juja.microservices.slack.archive.model.entity;


		public class Util {

		public static String getFile(String fileName){

34 arc f5 save slack channels #59

Are you sure you want to change the base?

34 arc f5 save slack channels #59

Conversation

NikolayNN commented Dec 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NikolayNN Dec 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NikolayNN Jan 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danilkuznetsov commented Jan 3, 2018 • edited Loading

NikolayNN Dec 27, 2017 •

edited

Loading

NikolayNN Jan 8, 2018 •

edited

Loading

danilkuznetsov commented Jan 3, 2018 •

edited

Loading